fix: Stripe publishable key should be accessible to non-admin user #6277

abhinavk96 · 2019-07-31T16:48:16Z

Short description of what this resolves:

Moves the stripe authorization relationship from EventSchema to EventPublicSchema, as normal users need publishable keys.
Use proper decorators for GET and POST requests in stripeAuhorizarion API.

Publishable API keys are meant solely to identify your account with Stripe, they aren’t secret. In other words, they can safely be published in places like your Stripe.js JavaScript code, or in an Android or iPhone app. Publishable keys only have the power to create tokens
Reference

stripe_auth_code (the other field in stripeAuhorizarion schema) has load_only=true, hence is never serialized in a GET request.

codecov · 2019-07-31T17:14:08Z

Codecov Report

Merging #6277 into development will decrease coverage by 0.03%.
The diff coverage is 57.14%.

@@               Coverage Diff               @@
##           development    #6277      +/-   ##
===============================================
- Coverage        65.57%   65.54%   -0.04%     
===============================================
  Files              286      286              
  Lines            14567    14588      +21     
===============================================
+ Hits              9552     9561       +9     
- Misses            5015     5027      +12

Impacted Files	Coverage Δ
app/api/schema/events.py	`91.79% <100%> (ø)`	⬆️
app/api/schema/stripe_authorization.py	`100% <100%> (ø)`	⬆️
app/api/stripe_authorization.py	`45.94% <36.84%> (-6.6%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ce62ff0...ebb8cf0. Read the comment docs.

iamareebjamal · 2019-07-31T17:21:08Z

StripeAuthorizationSchema may contain other sensitive keys as well? Or is it just public key

abhinavk96 · 2019-07-31T17:30:12Z

@iamareebjamal

stripe_auth_code (the other field in stripeAuhorizarion schema) has load_only=true, hence is never serialized in a GET request.

It contains an authcode, but that field is write only.

iamareebjamal · 2019-07-31T17:33:50Z

Still, it should not be shown to any user. Instead, StripeAuthorizationPublicSchema should be made which only includes public keys

houndci-bot · 2019-07-31T17:47:13Z

app/api/schema/stripe_authorization.py

+
+    stripe_auth_code = fields.Str(load_only=True, required=True)
+
+


blank line at end of file

abhinavk96 · 2019-07-31T17:57:10Z

@iamareebjamal done.

iamareebjamal · 2019-11-26T06:01:49Z

app/api/stripe_authorization.py

+    decorators = (api.has_permission('is_coorganizer', fetch="event_id",
+                                     fetch_as="event_id", model=StripeAuthorization),)


fossasia/open-event-frontend#3523 is happening due to this. The endpoint is /v1/stripe-authorizations, there is no event_id in view kwargs and hence it fails everytime it is accessed

@CosmicCoder96 Can this be removed or was it added for a specific reason. I can't see how it will work for /v1/stripe-authorizations endpoint. Maybe it is used in some other relations. Please clarify so that we don't break something else when removing this

@iamareebjamal It should be removed. Won't affect FE.

iamareebjamal · 2019-11-27T12:07:37Z

@kushthedude @codedsun Please handle this ASAP

codedsun · 2019-11-27T16:37:12Z

@iamareebjamal Do i have to remove the decorators?

iamareebjamal · 2019-11-27T16:40:29Z

Yes

codedsun · 2019-11-27T16:41:09Z

Sending a pr!

kushthedude · 2019-11-27T16:43:38Z

You have to test too if decorator is the issue or not.

…

On Wed, 27 Nov, 2019, 22:11 Suneet Srivastava, ***@***.***> wrote: Sending a pr! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#6277?email_source=notifications&email_token=AKQMTLUHHFI3AZIKWU5L5LTQV2PKNA5CNFSM4IIIT3N2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFKCZNI#issuecomment-559164597>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AKQMTLXTQ3QT5TESMSTAEBTQV2PKNANCNFSM4IIIT3NQ> .

codedsun · 2019-11-27T16:51:20Z

@kushthedude- @iamareebjamal has confirmed in the issue #6277 (comment)

abhinavk96 added the Priority: URGENT label Jul 31, 2019

abhinavk96 requested a review from iamareebjamal July 31, 2019 16:48

abhinavk96 force-pushed the alpha branch from c248ac6 to 2dbbb19 Compare July 31, 2019 16:50

abhinavk96 force-pushed the alpha branch from 2dbbb19 to 709a8c4 Compare July 31, 2019 17:47

houndci-bot reviewed Jul 31, 2019

View reviewed changes

app/api/schema/stripe_authorization.py

stripe_auth_code = fields.Str(load_only=True, required=True)

Copy link

houndci-bot Jul 31, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blank line at end of file

fix: Stripe publishable key should be accessible to user

ebb8cf0

abhinavk96 force-pushed the alpha branch from 709a8c4 to ebb8cf0 Compare July 31, 2019 17:57

iamareebjamal approved these changes Jul 31, 2019

View reviewed changes

abhinavk96 merged commit 1ef10f5 into fossasia:development Jul 31, 2019

iamareebjamal mentioned this pull request Aug 1, 2019

Stripe not working fossasia/open-event-attendee-android#2133

Closed

mrsaicharan1 pushed a commit to mrsaicharan1/open-event-server that referenced this pull request Aug 9, 2019

fix: Stripe publishable key should be accessible to user (fossasia#6277)

4aeba0b

iamareebjamal reviewed Nov 26, 2019

View reviewed changes

iamareebjamal mentioned this pull request Nov 26, 2019

Wizard Step 1: Cannot save (Publish) after adding Stripe - Error "Object not found" fossasia/open-event-frontend#3523

Closed

iamareebjamal mentioned this pull request Aug 5, 2020

Stripe publishable key should be accessible to non-admin user #7187

Closed


		stripe_auth_code = fields.Str(load_only=True, required=True)

		decorators = (api.has_permission('is_coorganizer', fetch="event_id",
		fetch_as="event_id", model=StripeAuthorization),)

fix: Stripe publishable key should be accessible to non-admin user #6277

fix: Stripe publishable key should be accessible to non-admin user #6277

Uh oh!

Conversation

abhinavk96 commented Jul 31, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Short description of what this resolves:

Uh oh!

codecov bot commented Jul 31, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

iamareebjamal commented Jul 31, 2019

Uh oh!

abhinavk96 commented Jul 31, 2019 via email • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iamareebjamal commented Jul 31, 2019

Uh oh!

houndci-bot Jul 31, 2019

Choose a reason for hiding this comment

Uh oh!

abhinavk96 commented Jul 31, 2019

Uh oh!

iamareebjamal Nov 26, 2019

Choose a reason for hiding this comment

Uh oh!

iamareebjamal Nov 26, 2019

Choose a reason for hiding this comment

Uh oh!

abhinavk96 Nov 27, 2019

Choose a reason for hiding this comment

Uh oh!

iamareebjamal commented Nov 27, 2019

Uh oh!

codedsun commented Nov 27, 2019

Uh oh!

iamareebjamal commented Nov 27, 2019

Uh oh!

codedsun commented Nov 27, 2019

Uh oh!

kushthedude commented Nov 27, 2019 via email

Uh oh!

codedsun commented Nov 27, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

abhinavk96 commented Jul 31, 2019 •

edited

Loading

codecov bot commented Jul 31, 2019 •

edited

Loading

abhinavk96 commented Jul 31, 2019 via email •

edited

Loading